Skip to content

nvme: keepalive fixups - #3998

Merged
babayet2 merged 3 commits into
microsoft:mainfrom
babayet2:fused-fixup
Jul 30, 2026
Merged

nvme: keepalive fixups#3998
babayet2 merged 3 commits into
microsoft:mainfrom
babayet2:fused-fixup

Conversation

@babayet2

Copy link
Copy Markdown
Collaborator

Improves the keepalive tests to actually run servicing, and adds a panic when there is a write to the admin queue in fused mode

@babayet2
babayet2 requested review from a team as code owners July 22, 2026 01:27
Copilot AI review requested due to automatic review settings July 22, 2026 01:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the OpenHCL NVMe keepalive servicing coverage by making the fused-keepalive test actually cross a servicing boundary, and adds a restore-time invariant check for fused keepalive devices to detect unexpected admin-queue activity.

Changes:

  • Update the fused keepalive NVMe test to restart OpenHCL with NVMe keepalive enabled and re-validate fused vs non-fused behavior post-servicing.
  • Add a restore-time check that panics if a fused-keepalive device is restored with a non-empty admin queue.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs Extends the fused keepalive test to perform an OpenHCL restart (servicing) with NVMe keepalive enabled and re-check device fusion state afterward.
vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs Adds a fused-keepalive restore-time assertion by panicking when pending admin commands exist during admin queue restore.

Comment on lines +910 to +916
if fused_keepalive_device && pending_commands_count > 0 {
panic!(
"fused keepalive device {pci_id} restored with a non-empty admin \
queue ({pending_commands_count} pending commands); fused devices \
must not issue admin commands after init"
);
}
@github-actions

Copy link
Copy Markdown

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad2f5a78-7441-43d3-9679-9de10f68a9c3
Copilot AI review requested due to automatic review settings July 22, 2026 05:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@github-actions

Copy link
Copy Markdown

@babayet2 babayet2 changed the title nvme: fixup for keepalive tests nvme: keepalive fixups Jul 30, 2026
@babayet2 babayet2 added the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Jul 30, 2026
@babayet2

Copy link
Copy Markdown
Collaborator Author

@mattkur the panics in this PR are already in 1.7, we want it in main, and backported to 1.8

@babayet2
babayet2 merged commit 191b331 into microsoft:main Jul 30, 2026
129 of 130 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport_1.8.2607 Change should be backported to the release/1.8.2607 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants